it doesn't sound right that internal and backend components depend on apimodels which are meant for the http API between frontend (api server) and clients.

Some suggestions:

1. We should be able to define internal errors separate from the APIs, and then implement interceptors that can translate internal errors to http errors
2. At the same time we should avoid having to translate each individual error to http format. So maybe having an interface that each of these errors implement, or a common underlying error type can simplify the translation
3. We should avoid having a central pkg that defines all internal errors in the system like what we have today with bacerror

Done, please let me know your thoughts about it.

We should also return:

• RequestID to help debug and grep server logs for the failed request
• ErrorCode that uniquely identify the error error without having to parse the message, similar to AWS

Keep in mind that some of this information are already in the http headers, such as status code and request id. It might be redundant to serialize them in the APIError. Maybe we should omit serializing them, and just have the client set those values from the headers.

I have added the RequestID. However, adding a bacalhau specific error code is tricky at this point. Mainly because we are need to make sure that we properly propagate error codes correctly every where. In addition, we need to think about how to formulate error codes for errors that occur before even we reach to bacalhau specific components, for example echo server middlewares.

I think we should pick that up in follow ups. Let me know what you think

Yeah please feel free to do this in a follow up PR. What I had in mind is something similar to the code below. The idea is to have standard error codes that can be used by our core components and even the modules if the errors apply, while still enabling the modules to define their own specific error code, such as BucketNotFound and ContainerStartFailed

The HTTPCode can be explicitly provided, inferred from the error code if it is a standard one, or fallback to 500.

What I really liked in your PR is the introduction of Component field which helps identify the source of the error and also removes the need for these error codes to be globally unique. I would just make the components more readable and descriptive, such as S3Publisher, Docker instead of abbreviations. To simplify things, I am proposing the default component to be Bacalhau, which doesn't mean much, but will allow us to incrementally improve the coverage of better errors.

Also as we progress, we should allow components to override the Component piece of the errors. Meaning a job not found error with JobStore component doesn't help much compared to knowing the component that actually called the job store. We don't need to do these changes today, but just thinking ahead.

What do you think?

const (
    BadRequest ErrorCode = "BadRequest"
    Unauthorized ErrorCode = "Unauthorized"
    ResourceNotFound ErrorCode = "ResourceNotFound"
    ResourceInUse ErrorCode = "ResourceInUse"
    ValidationFailed ErrorCode = "ValidationFailed"
    VersionMismatch ErrorCode = "VersionMismatch"
    TooManyRequests ErrorCode = "TooManyRequests"
    
    InternalError ErrorCode = "InternalError"
    NotImplemented ErrorCode = "NotImplemented"
    ServiceUnavailable ErrorCode = "ServiceUnavailable"
    DatastoreFailure ErrorCode = "DatastoreFailure"
    NetworkFailure ErrorCode = "NetworkFailure"
    ConfigurationError ErrorCode = "ConfigurationError"
    ResourceExhausted ErrorCode = "ResourceExhausted"
)

// Note: Component-specific error codes should be defined in their respective packages.
// Examples might include:
// - BucketNotFound
// - ObjectNotFound
// - InvalidCredentials
// - ImageNotFound
// - ContainerStartFailed
// - PeerConnectionFailed

type BaseError struct {
    Code           ErrorCode         `json:"code"`
    Message        string            `json:"message"`
    Component      string            `json:"component"`
    HTTPCode       int               `json:"http_code"`
    Hint           string            `json:"hint,omitempty"`
    Retryable      bool              `json:"retryable"`
    FailsExecution bool              `json:"fails_execution"`
    Details        map[string]string `json:"details,omitempty"`
}

func NewBaseError(format string, a ...any) *BaseError {
    return &BaseError{
        Code:      InternalError,
        Message:   fmt.Sprintf(format, a...),
        Component: "Bacalhau",
        HTTPCode:  0, // Will be inferred later if not set explicitly
    }
}

func (e *BaseError) Error() string {
    return fmt.Sprintf("[%s] %s: %s", e.Code, e.Component, e.Message)
}

func (e *BaseError) WithErrorCode(code ErrorCode) *BaseError {
    e.Code = code
    return e
}

func (e *BaseError) WithHTTPCode(httpCode int) *BaseError {
    e.HTTPCode = httpCode
    return e
}

func (e *BaseError) HTTPStatusCode() int {
    if e.HTTPCode != 0 {
        return e.HTTPCode
    }
    return inferHTTPStatusCode(e.Code)
}

func inferHTTPStatusCode(code ErrorCode) int {
    switch code {
    case BadRequest, ValidationFailed:
        return http.StatusBadRequest
    case Unauthorized:
        return http.StatusUnauthorized
    case ResourceNotFound:
        return http.StatusNotFound
    case ResourceInUse, VersionMismatch:
        return http.StatusConflict
    case TooManyRequests:
        return http.StatusTooManyRequests
    case NotImplemented:
        return http.StatusNotImplemented
    case ServiceUnavailable:
        return http.StatusServiceUnavailable
    default:
        return http.StatusInternalServerError
    }
}

// rest of the methods

nit: this should fit in boltdb pkg as it is related to the job store implementation. I understand I asked to define and use errors mainly in the jobstore pkg, but that should be for common errors that apply to different implementations

Done

nit: you might want to define RequestID and HTTPStatusCode as transient fields so you can avoid serializing them and then populate them in the client from the headers. This is doable by using json:- tag

Done

We already have some logic in enabling debug mode in our http server, but they are not consistent yet, and maybe we should unify them and avoid adding a third mode

I would say we can just rely on the LogLevel configuration, and if it is debug or trace, then we set Router.Debug = true

In your error handler you can then do if c.Echo.Debug == true to check if debug mode is enabled in the router

Done

nit: I would suggest to pass the actual error instead of just the string. This will allow NewBoltDbError to intercept the error and return different types or codes based on the actual error.

My not be needed here or now, but I am just thinking ahead when we capture and throw errors for docker executor and s3 publisher for example. You might find this pattern useful to centralize intercepting errors per component

Done